Code Review: TUnitAssertions0016 — Collection IsEqualTo Reference Equality

Good addition overall. The structure follows existing analyzer patterns well, the code fix is correct and trivia-preserving, and the test matrix is solid. A few things worth discussing:

Design Concern: False Positives for Custom Collections with Value Equality

The core issue: IsCollectionWithoutStructuralEquality checks whether a type implements IEnumerable, but it does not check whether the type overrides Equals. This means the analyzer will fire on any custom collection that does implement structural equality via an Equals override:

public class MyOrderedSet : IEnumerable<int>
{
    public override bool Equals(object? obj) => /* structural comparison */;
    // ...
}

await Assert.That(new MyOrderedSet()).IsEqualTo(new MyOrderedSet()); // ← flagged as Info, but wrong!

The method name IsCollectionWithoutStructuralEquality implies it checks for absence of structural equality, but it actually only checks for presence of IEnumerable. The fix is to also verify that Equals is not overridden beyond object:

private static bool IsCollectionWithoutStructuralEquality(ITypeSymbol type, INamedTypeSymbol ienumerable)
{
    // ... existing exclusions ...

    if (!ImplementsIEnumerable(type, ienumerable)) return false;

    // Only flag if Equals is not overridden (i.e., reference equality is used)
    return !HasEqualsOverride(type);
}

private static bool HasEqualsOverride(ITypeSymbol type)
{
    var current = type;
    while (current is not null && current.SpecialType != SpecialType.System_Object)
    {
        if (current.GetMembers("Equals").OfType<IMethodSymbol>()
            .Any(m => m.IsOverride && m.Parameters.Length == 1 &&
                      m.Parameters[0].Type.SpecialType == SpecialType.System_Object))
            return true;
        current = current.BaseType;
    }
    return false;
}

This would make the diagnostic semantically accurate and eliminate false positives.

Missing Test: Custom IEnumerable with Equals Override

There is no test covering the false-positive case described above — a custom IEnumerable-implementing type that overrides Equals. Adding that test would both document the intended behavior and guard against regressions.

Minor: Resources.resx Missing Trailing Newline

The diff shows \ No newline at end of file for Resources.resx. Other entries in that file likely have a trailing newline; this should be consistent.

Observation: CompilationStart for IEnumerable Symbol Caching

Wrapping the operation action in RegisterCompilationStartAction to cache the IEnumerable symbol is a good pattern — it avoids re-resolving the symbol on every invocation. This is better than the approach in some other analyzers (e.g., PreferIsNullAnalyzer) that don't cache. Worth keeping, and potentially worth applying to other analyzers in a follow-up.

Summary

The fix for the blocking issue is straightforward: add an Equals-override check inside IsCollectionWithoutStructuralEquality. Everything else — the code fix provider, the span reporting, the chained-call preservation, the exclusions for string/Memory/Span — looks correct.

Code Review: TUnitAssertions0016 — Follow-up (v2)

Previous Review vs. Current Code

My prior review flagged three issues. Looking at the current diff carefully, all three were already present and handled in the original submission — the review was inaccurate. To set the record straight:

Apologies for the noise in the prior review.

New Concern: IEquatable<T> Gap

OverridesObjectEquals checks for Equals(object) overrides but not for IEquatable\<T\> implementation. This matters because EqualityComparer\<T\>.Default (which TUnit's IsEqualTo likely uses) prefers IEquatable\<T\>.Equals when implemented, making the comparison structural — not reference-based.

A type like this would be falsely flagged:

public class MyCollection : IEnumerable<int>, IEquatable<MyCollection>
{
    public bool Equals(MyCollection? other) => /* structural */;
    // Does NOT override Equals(object) separately
}

await Assert.That(new MyCollection()).IsEqualTo(new MyCollection()); // ← falsely flagged

Fix — extend the guard in IsCollectionWithoutStructuralEquality:

private static bool ImplementsIEquatableOfSelf(INamedTypeSymbol type)
{
    return type.AllInterfaces.Any(i =>
        i.Name == "IEquatable" &&
        i.TypeArguments.Length == 1 &&
        SymbolEqualityComparer.Default.Equals(i.TypeArguments[0], type));
}

And add || ImplementsIEquatableOfSelf(...)" to the return condition alongside OverridesObjectEquals`.

This is a low-probability edge case (most types that implement IEquatable\<T\> also override Equals(object)), but worth closing for correctness.

Everything Else Looks Good

• RegistrationCompilationStartAction + caching the IEnumerable symbol: correct and efficient.
• Span reporting (TextSpan.FromBounds) is precise — highlights only IsEqualTo(...) rather than the full chain.
• Code fix correctly replaces the identifier via SyntaxFactory.IdentifierName with trivia preserved, leaving chained calls intact.
• Exclusions for string, Memory\<T\>, Span\<T\>, ReadOnlySpan\<T\>, ReadOnlyMemory\<T\> are correct and tested.
• Record exclusion (Record_Collection_Not_Flagged) correctly handles synthesised Equals from record types.
• WellKnownFixAllProviders.BatchFixer is the right choice for a simple rename fix.

The only actionable gap is the IEquatable\<T\> case. Given that DiagnosticSeverity.Info keeps this non-blocking for users, this is non-blocking for the PR as well — but worth a follow-up or inline fix.

Code Review: TUnitAssertions0016 — v3 (current state)

Previous Reviews vs. Current State

The PR has evolved through two prior rounds of review feedback:

Both concerns are now correctly handled and tested. The analyzer properly skips collections that implement structural equality via either mechanism.

Current Code Assessment

IsCollectionWithoutStructuralEquality now correctly returns false when:

• The type is string, Memory<T>, ReadOnlyMemory<T>, Span<T>, or ReadOnlySpan<T>
• The type does not implement IEnumerable
• The type overrides Equals(object)
• The type implements IEquatable<T> of itself

This is semantically accurate. The return condition !OverridesObjectEquals(type) && !ImplementsIEquatableOfSelf(type) is correct and complete.

Minor Observation: Redundant Span/Memory Exclusions

Span<T> and ReadOnlySpan<T> are ref structs and cannot implement interfaces — so ImplementsIEnumerable would already return false for them. Similarly, Memory<T> and ReadOnlyMemory<T> do not implement IEnumerable. The explicit exclusions are technically redundant, but they serve as clear documentation of intent and add no cost. Not worth changing.

Everything Looks Good

• Analyzer logic: Sound. The CompilationStartAction pattern for caching the IEnumerable symbol is correct and efficient.
• Span reporting: TextSpan.FromBounds(memberAccess.Name.SpanStart, invocationSyntax.Span.End) highlights only IsEqualTo(...) rather than the full chain. Precise and correct.
• Code fix: Simple identifier rename via SyntaxFactory.IdentifierName with WithTriviaFrom is the right approach. Preserves chained calls and does not require any AST restructuring.
• GetFixAllProvider: WellKnownFixAllProviders.BatchFixer is correct for a pure rename fix.
• Test coverage: All meaningful cases are covered — List, array, string, int, Count() overload, record, IEquatable, Equals override.

This is ready to merge.